-
Notifications
You must be signed in to change notification settings - Fork 13.5k
More robustly deal with relaxed bounds and improve their diagnostics #142693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
8d31700
to
4634541
Compare
69e237d
to
03d01fa
Compare
fff5f99
to
8a6f65d
Compare
Only relevant to the internal feature `more_maybe_bounds`.
We no longer move trait predicates where the self ty is a ty param to "the bounds of a ty param".
* The phrasing "only does something for" made sense back when this diagnostic was a (hard) warning. Now however, it's simply a hard error and thus completely rules out. * The primary message was way too long * The new wording more closely mirrors the wording we use for applying other bound modifiers (like `const` and `async`) to incompatible traits. * "all other traits are not bound by default" is no longer accurate under Sized Hierarchy. E.g., traits and assoc tys are (currently) bounded by `MetaSized` by default but can't be relaxed using `?MetaSized` (instead, you relax it by adding `PointeeSized`). * I've decided against adding any diagnositic notes or suggestions for now like "trait `Trait` can't be relaxed as it's not bound by default" /which would be incorrect for `MetaSized` and assoc tys as mentioned above) or "consider changing `?MetaSized` to `PointeeSized`" as the Sized Hierarchy impl is still WIP)
Having multiple relaxed bounds like `?Sized + ?Iterator` is actually *fine*. We actually want to reject *duplicate* relaxed bounds like `?Sized + ?Sized` because these most certainly represent a user error. Note that this doesn't mean that we accept more code because a bound like `?Iterator` is still invalid as it's not relaxing a *default* trait and the only way to define / use more default bounds is under the experimental and internal feature `more_maybe_bounds` plus `lang_items` plus unstable flag `-Zexperimental-default-bounds`. Ultimately, this simply *reframes* the diagnostic. The scope of `more_maybe_bounds` / `-Zexperimental-default-bounds` remains unchanged as well.
8a6f65d
to
04dad4e
Compare
@bors2 try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (bfa10de): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (secondary -2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.9%, secondary 4.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 1.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 692.606s -> 692.191s (-0.06%) |
Scaffolding for #135229 (CC #135331)
Fixes #136944.
Fixes #142718.